Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parallexaxis doc #249

Merged
merged 8 commits into from
Apr 5, 2023
Merged

Parallexaxis doc #249

merged 8 commits into from
Apr 5, 2023

Conversation

quaquel
Copy link
Owner

@quaquel quaquel commented Apr 5, 2023

closes #141

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@quaquel quaquel requested a review from EwoutH April 5, 2023 07:23
@quaquel quaquel self-assigned this Apr 5, 2023
@quaquel quaquel added the docs label Apr 5, 2023
@quaquel quaquel added this to the 2.4.0 milestone Apr 5, 2023
Copy link
Collaborator

@EwoutH EwoutH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there is overlap between the Parameters and Attributes sections. I don't think this is ideal. See also the readthedocs build.

Did you actually change anything on the outputspace_exploration_lakeproblem.ipynb? If not, please clean up (by discarding those changes)

Also please make the PR title more descriptive (and in imperative mood).


Attributes
----------
limits : DataFrame
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a description of limits

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added the description from the parameters part

set of Axes that are to be shown flipped
axis_labels : list of str
fontsize : int
normalizer : MinMaxScaler instance
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a description of normalizer

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this an implementation detail and removed it from the documentation. It is not part of the public API so you don't want to mess with the normalizer.

labels associated with lines


The basic setup of the Parallel Axis plot is a row of mpl Axes instances, with all whitespace
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this line have a header, since from here class methods are discussed?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved it into Notes which is the proper place for implementation details

@quaquel
Copy link
Owner Author

quaquel commented Apr 5, 2023

I can't figure out how to discard that change. It indeed should not be there. I'll fix the rest.

Note that the attributes and parameters can overlap. Parameters documents the arguments for the __init__ while attributes documents the state of a class.

@quaquel quaquel requested a review from EwoutH April 5, 2023 08:57
@EwoutH
Copy link
Collaborator

EwoutH commented Apr 5, 2023

Looks good!

Please squash while merging and cleanup the commit message and body while merging.

@quaquel quaquel merged commit f5023af into master Apr 5, 2023
@quaquel quaquel deleted the parallexaxis_doc branch April 5, 2023 12:14
quaquel added a commit that referenced this pull request Apr 5, 2023
Details the attributes available on ParallelAxis. 

closes #141
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow changing the figure size of a parcoords.ParallelAxes() plot
2 participants